Skip to content

[WC-3464] Rich text Tiptapeditor#2270

Open
gjulivan wants to merge 3 commits into
mainfrom
rich-text/tiptapeditor
Open

[WC-3464] Rich text Tiptapeditor#2270
gjulivan wants to merge 3 commits into
mainfrom
rich-text/tiptapeditor

Conversation

@gjulivan

Copy link
Copy Markdown
Collaborator

Pull request type


Description

@gjulivan
gjulivan requested a review from a team as a code owner June 17, 2026 11:49
@gjulivan
gjulivan force-pushed the rich-text/tiptapeditor branch 2 times, most recently from 584fe21 to 96df6fb Compare July 1, 2026 08:34
@github-actions

This comment has been minimized.

@gjulivan
gjulivan force-pushed the rich-text/tiptapeditor branch from 96df6fb to 57a6695 Compare July 1, 2026 08:53
@github-actions

This comment has been minimized.

@gjulivan
gjulivan force-pushed the rich-text/tiptapeditor branch from 57a6695 to 3ca1f8c Compare July 2, 2026 21:56
@github-actions

This comment has been minimized.

@gjulivan
gjulivan force-pushed the rich-text/tiptapeditor branch 8 times, most recently from 32f038c to e544a1d Compare July 13, 2026 00:08
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@gjulivan
gjulivan force-pushed the rich-text/tiptapeditor branch from c4bbd17 to 6397d1b Compare July 13, 2026 20:17
@github-actions

This comment has been minimized.

@yordan-st

Copy link
Copy Markdown
Contributor

Would have a look at the high and medium flagged CC review comments

@gjulivan
gjulivan force-pushed the rich-text/tiptapeditor branch 4 times, most recently from f7d16ca to 2755b5d Compare July 21, 2026 11:15
@gjulivan
gjulivan force-pushed the rich-text/tiptapeditor branch 3 times, most recently from 5925c7a to bd5d88b Compare July 24, 2026 10:58
@github-actions

This comment has been minimized.

@gjulivan
gjulivan force-pushed the rich-text/tiptapeditor branch from bd5d88b to 6d6c750 Compare July 24, 2026 11:24
@github-actions

This comment has been minimized.

@gjulivan
gjulivan force-pushed the rich-text/tiptapeditor branch from 6d6c750 to cd882a6 Compare July 24, 2026 11:49
@github-actions

This comment has been minimized.

@gjulivan
gjulivan force-pushed the rich-text/tiptapeditor branch from cd882a6 to 9bf1d12 Compare July 24, 2026 12:28
@github-actions

This comment has been minimized.

@gjulivan
gjulivan force-pushed the rich-text/tiptapeditor branch 2 times, most recently from 469eaa6 to aa56ad2 Compare July 24, 2026 17:10
@github-actions

This comment has been minimized.

@gjulivan
gjulivan force-pushed the rich-text/tiptapeditor branch 3 times, most recently from 87a4a10 to 8853e96 Compare July 27, 2026 09:07
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@gjulivan
gjulivan force-pushed the rich-text/tiptapeditor branch 4 times, most recently from fbf5d78 to f12a929 Compare July 27, 2026 12:54
@gjulivan
gjulivan force-pushed the rich-text/tiptapeditor branch 2 times, most recently from 5da8c04 to 844971a Compare July 27, 2026 12:57
@gjulivan
gjulivan force-pushed the rich-text/tiptapeditor branch from 844971a to e97de41 Compare July 27, 2026 13:58

// Blur the editor to trigger the save/normalize path.
await page.keyboard.press("Tab");
await page.waitForTimeout(500);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔶 Medium — Hardcoded waitForTimeout delay

page.waitForTimeout(500) is a hardcoded sleep. Per e2e-test-guidelines, replace it with a web-first assertion that resolves as soon as the condition is met.

After pressing Tab to blur, the editor's empty-content normalisation is synchronous within Tiptap's state flush. A reliable wait is:

Suggested change
await page.waitForTimeout(500);
await expect(editor).toHaveAttribute("data-placeholder");

Or, if you want to assert the save has propagated to the Mendix attribute, wait for the text-content assertion itself (Playwright retries automatically):

        await page.keyboard.press("Tab");
        await expect(editor).toContainText("");

Either avoids the 500 ms wall-clock cost and the flakiness risk of a fixed sleep.

await page.goto("/p/classmode");
await waitForMendixApp(page);
await expect(page.locator(".mx-name-richText1")).toBeVisible();
await expect(page.locator(".mx-name-richText1")).toHaveScreenshot(`classModeEditor.png`, { threshold: 0.4 });

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔶 Medium — Missing screenshot baselines for new class-mode tests

classModeEditor.png and classModeViewCodeDialog.png (line 170) are referenced by toHaveScreenshot but their baseline PNGs are not committed to the snapshots directory. Playwright will fail on first run because there is nothing to compare against.

Per the E2E guidelines, baseline images must be committed with the spec. Generate and commit them before merging:

cd packages/pluggableWidgets/rich-text-web
pnpm run e2e --update-snapshots
git add e2e/RichText.spec.js-snapshots/classModeEditor-chromium-linux.png \
         e2e/RichText.spec.js-snapshots/classModeViewCodeDialog-chromium-linux.png

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants